fix: only apply limit disclosure for matching vcs #168
Conversation
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
|
Another one @sanderPostma / @ksadjad :) We're just now basically starting to explore multi-vp / multi-cred exchanges, and that results in some edge cases being hit |
| import PexMessages from '../../types/Messages'; | ||
| import { applySdJwtLimitDisclosure, JsonPathUtils } from '../../utils'; | ||
| import { EvaluationClient } from '../evaluationClient'; | ||
|
|
||
| import { AbstractEvaluationHandler } from './abstractEvaluationHandler'; | ||
| import { elligibleInputDescriptorsForWrappedVc } from './markForSubmissionEvaluationHandler'; |
There was a problem hiding this comment.
"elligible" is consistently misspelled
-> eligible
|
Hey @sanderPostma, has this been released as an unstable release of PEX? It seems the latest (unstable 7) doesn't include this fix yet |
|
Hi @TimoGlastra Sadjad started doing some fixes, but to make the lib work for singleton or one-element-array VPs I have made an emergency release from another branch (develop-20240918) so we could build our other repos again, but the multi-vp tests are failing in that one. |
When you have a presentation definition with multiple input descriptors, and multiple VCs, it tries to apply the limit disclosure to all VCs. This means that claims are not disclosed that do need to be disclosed.
This PR updates the limit disclosure handler to only apply limit disclosure based on constraints in an input descriptor if the VC matches the requirements of the input descriptor.
It also handles an advance case, where if one VC matches multiple input descriptors. This is handled by combining the constraints and disclosing both requirements. If you only use the VC for one input descriptor in the end it means you disclose some attribute that are not needed, but as this is already an advanced case I think this is fine for now (we at least disclose what is needed). With the current API it would be very hard to fix this, as we need to have a mapping of input descriptor -> disclosed VC. And then maybe just include the same VC twice with different disclosures. But for now I think this already fixes the main bug.